Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: handle FlowControllerTest thread start variation by polling #1586

Merged
merged 6 commits into from
Apr 17, 2023

Conversation

burkedavison
Copy link
Member

Fixes #1286

Issue: "Expected thread to be WAITING (blocked) but instead was RUNNABLE (had not been started yet, or had not yet reached the blocking logic.)"

This PR increases the amount of time spent waiting for the thread to be blocked, while also implementing a polling assertion mechanism to prevent waiting the maximum amount of time.

Also: Raw types fixed in this file.

@burkedavison burkedavison requested a review from a team as a code owner March 30, 2023 13:23
@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Mar 30, 2023
@burkedavison
Copy link
Member Author

@suztomo , @blakeli0 , and/or @meltsufin : PTAL when you have a moment.

Stopwatch stopwatch = Stopwatch.createStarted();
while (true) {
try {
assertion.run();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to have: Inspired by your test, maybe we can have a counter that counts how many times we tried before a successful assertion?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you have in mind for how it would be used? When successful, it currently proceeds without logging anything.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you have in mind for how it would be used? When successful, it currently proceeds without logging anything.

We can add logs to show that how many times we tried when successful, so that it could be useful if we want to check previously successful runs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I smell YAGNI here.

When we really want to measure certain operations, I think we would write a simple Java code to run the operation. Then we would run it against old code by checking out release tags via Git.

@product-auto-label product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Apr 14, 2023
@product-auto-label product-auto-label bot added size: m Pull request size is medium. and removed size: l Pull request size is large. labels Apr 14, 2023
@sonarqubecloud
Copy link

[gapic-generator-java-root] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@sonarqubecloud
Copy link

[java_showcase_integration_tests] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@sonarqubecloud
Copy link

[java_showcase_unit_tests] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@burkedavison burkedavison merged commit 0019c92 into main Apr 17, 2023
@burkedavison burkedavison deleted the FlowControllerTest-polling branch April 17, 2023 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gax-java(windows): Flaky FlowControllerTest.testNumberOfBytesOutOfBoundaryWontDeadlock
3 participants